-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(userstatus): trigger absence start job even for absences starting in the past #47183
Conversation
… in the past Signed-off-by: Anna Larch <anna@nextcloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this change.
I deliberately chose not to emit the start event when an absence period already started in the past. An app may always listen to OutOfOfficeScheduledEvent
or OutOfOfficeChangedEvent
to handle those cases. So that the OutOfOfficeStartedEvent
is reserved for when a scheduled absence starts without user interaction.
In a sense this could be seen as a breaking change. Although I'm not sure if there actually is a use case that would break (at least in our code).
$runJobAt = ($eventData->getStartDate() < $now) ? $now : $eventData->getStartDate(); | ||
$this->jobList->scheduleAfter( | ||
OutOfOfficeEventDispatcherJob::class, | ||
$runJobAt, | ||
[ | ||
'id' => $absence->getId(), | ||
'event' => OutOfOfficeEventDispatcherJob::EVENT_START, | ||
], | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$runJobAt = ($eventData->getStartDate() < $now) ? $now : $eventData->getStartDate(); | |
$this->jobList->scheduleAfter( | |
OutOfOfficeEventDispatcherJob::class, | |
$runJobAt, | |
[ | |
'id' => $absence->getId(), | |
'event' => OutOfOfficeEventDispatcherJob::EVENT_START, | |
], | |
); | |
$this->jobList->scheduleAfter( | |
OutOfOfficeEventDispatcherJob::class, | |
$eventData->getStartDate(), | |
[ | |
'id' => $absence->getId(), | |
'event' => OutOfOfficeEventDispatcherJob::EVENT_START, | |
], | |
); |
Can always use getStartDate()
here as the job will be picked up if it is in the past IIRC.
good point, that is indeed something to be considered. Although a case could be made that an event should be thrown regardless as it is indeed starting right now, so event listeners could do their thing. |
Superseded by #47201 |
Fixes #47058
Summary
The absence start job was not created for absences in the past - which could also mean an absence starting today (absence timestamp is midnight, but time for
$now
is larger).Since absences that start in the past and are still ongoing should be considered nonetheless, absences are now only evaluated by their end date. if that is in the past, no jobs are created.
Checklist
Screenshots before/after for front-end changesDocumentation (manuals or wiki) has been updated or is not required